-
Notifications
You must be signed in to change notification settings - Fork 40
grpc-native: Support client calls on native #452
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, great work! Thanks for the amount of comments and the big test set, it was very pleasant to read!
I have some comments and suggestions below, please address them before merging.
One big question that I have, is using C lib instead of C++ one - how hard it would be for use to support gRPC features as Auth, interceptors, etc? It is available is C API?
grpc/grpc-core/src/commonTest/kotlin/kotlinx/rpc/grpc/test/RawClientTest.kt
Outdated
Show resolved
Hide resolved
grpc/grpc-core/src/commonTest/kotlin/kotlinx/rpc/grpc/test/RawClientTest.kt
Show resolved
Hide resolved
grpc/grpc-core/src/nativeMain/kotlin/kotlinx/rpc/grpc/internal/CallbackFuture.kt
Outdated
Show resolved
Hide resolved
grpc/grpc-core/src/nativeMain/kotlin/kotlinx/rpc/grpc/internal/CompletionQueue.kt
Outdated
Show resolved
Hide resolved
grpc/grpc-core/src/nativeMain/kotlin/kotlinx/rpc/grpc/internal/CompletionQueue.kt
Outdated
Show resolved
Hide resolved
grpc/grpc-core/src/nativeMain/kotlin/kotlinx/rpc/grpc/internal/NativeManagedChannel.kt
Outdated
Show resolved
Hide resolved
grpc/grpc-core/src/nativeMain/kotlin/kotlinx/rpc/grpc/internal/utils.kt
Outdated
Show resolved
Hide resolved
grpc/grpc-core/src/nativeMain/kotlin/kotlinx/rpc/grpc/ManagedChannel.native.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Please look at the comments below
Also, please revisit all places where you do check
and replace with internalError
if needed, I saw at least one such place
grpc/grpc-core/src/nativeMain/kotlin/kotlinx/rpc/grpc/internal/CompletionQueue.kt
Show resolved
Hide resolved
grpc/grpc-core/src/nativeMain/kotlin/kotlinx/rpc/grpc/internal/CompletionQueue.kt
Outdated
Show resolved
Hide resolved
grpc/grpc-core/src/nativeMain/kotlin/kotlinx/rpc/grpc/internal/NativeClientCall.kt
Show resolved
Hide resolved
grpc/grpc-core/src/nativeMain/kotlin/kotlinx/rpc/grpc/ManagedChannel.native.kt
Outdated
Show resolved
Hide resolved
grpc/grpc-core/src/nativeMain/kotlin/kotlinx/rpc/grpc/ManagedChannel.native.kt
Show resolved
Hide resolved
Signed-off-by: Johannes Zottele <[email protected]>
Signed-off-by: Johannes Zottele <[email protected]>
Signed-off-by: Johannes Zottele <[email protected]>
Signed-off-by: Johannes Zottele <[email protected]>
Signed-off-by: Johannes Zottele <[email protected]>
Signed-off-by: Johannes Zottele <[email protected]>
Signed-off-by: Johannes Zottele <[email protected]>
Signed-off-by: Johannes Zottele <[email protected]>
Signed-off-by: Johannes Zottele <[email protected]>
Signed-off-by: Johannes Zottele <[email protected]>
Signed-off-by: Johannes Zottele <[email protected]>
Signed-off-by: Johannes Zottele <[email protected]>
Signed-off-by: Johannes Zottele <[email protected]>
Signed-off-by: Johannes Zottele <[email protected]>
c5fd51e
to
10f4c7b
Compare
* grpc-native: Initial client setup Signed-off-by: Johannes Zottele <[email protected]> * grpc-native: Working CompletionQueue Signed-off-by: Johannes Zottele <[email protected]> * grpc-native: Working version of NativeClientCall Signed-off-by: Johannes Zottele <[email protected]> * grpc-native: Working rewrite state Signed-off-by: Johannes Zottele <[email protected]> * grpc-native: Add callback future Signed-off-by: Johannes Zottele <[email protected]> * grpc-native: Refactor to use callbacks instead of coroutines Signed-off-by: Johannes Zottele <[email protected]> * grpc-native: Fixes after rebase Signed-off-by: Johannes Zottele <[email protected]> * grpc-native: Implement bridge to common Signed-off-by: Johannes Zottele <[email protected]> * grpc-native: Remove unnecessary C header definitions Signed-off-by: Johannes Zottele <[email protected]> * grpc-native: Rename grpcpp_c to kgrpc Signed-off-by: Johannes Zottele <[email protected]> * grpc-native: Reduce library dependencies (fixes KRPC-185) Signed-off-by: Johannes Zottele <[email protected]> * grpc-native: Write docs Signed-off-by: Johannes Zottele <[email protected]> * grpc-native: Address PR comments Signed-off-by: Johannes Zottele <[email protected]> * grpc-native: Address PR comments Signed-off-by: Johannes Zottele <[email protected]> --------- Signed-off-by: Johannes Zottele <[email protected]>
Subsystem
gRPC/Native
Solution
This PR uses the grpc-core (C) library to support client calls for Kotin/Native.